Skip to content

feat: project telemetry config models #1972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 4, 2025

Conversation

jonaro00
Copy link
Member

No description provided.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR introduces telemetry configuration models to support external telemetry sink integrations with Betterstack, Datadog, and Grafana Cloud services.

  • Added telemetry.rs with ProjectTelemetrySinkConfig enum for handling different telemetry provider configurations
  • Implemented From<Vec<ProjectTelemetrySinkConfig>> trait for safe display representation in ProjectTelemetryConfigResponse
  • Added TypeScript type definitions in types.ts with corresponding interfaces and discriminated unions for telemetry configurations

3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

This PR updates the telemetry configuration models with improved field accessibility and enum handling patterns.

  • Made configuration fields public (pub) in BetterstackConfig, DatadogConfig, and GrafanaCloudConfig structs for external access
  • Added strum::AsRefStr derive macro to ProjectTelemetrySinkConfig enum for string conversion support
  • Simplified enum pattern matching from { .. } to (_) for cleaner code in From implementation

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

This PR adds the final touches to the telemetry configuration models with improved type safety and database integration.

  • Added #[typeshare::typeshare] attribute to all telemetry structs and enums for TypeScript type generation
  • Implemented as_db_type() method on ProjectTelemetrySinkConfig for database type string generation
  • Added TelemetrySinkStatus struct with enabled field for safe status representation

1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

This PR updates the telemetry configuration models to ensure consistent naming conventions between Rust and TypeScript.

  • Changed TypeScript type discriminators in types.ts from PascalCase to snake_case for consistency with Rust enums
  • Added #[serde(rename_all = "snake_case")] to ensure consistent serialization across languages
  • Added #[strum(serialize_all = "snake_case")] for string conversion consistency

2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

This PR updates the typeshare-cli version to 1.13.0 across build configurations to support the new telemetry models.

  • Updated typeshare-cli version from 1.11.0 to 1.13.0 in Makefile.toml for local development
  • Updated typeshare-cli version in .circleci/config.yml for CI/CD pipeline consistency
  • Regenerated types.ts with updated typeshare version to ensure type compatibility

4 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

This PR simplifies telemetry type names by removing redundant 'Project' prefixes while maintaining functionality.

  • Renamed ProjectTelemetryConfigResponse to TelemetryConfigResponse in both Rust and TypeScript
  • Renamed ProjectTelemetrySinkConfig to TelemetrySinkConfig for cleaner naming
  • Updated type references in telemetry.rs and types.ts to reflect new names

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

Based on the latest changes and previous reviews, I'll provide a summary focusing only on the most recent updates:

This PR finalizes the telemetry configuration models with improved type naming and consistent serialization patterns.

  • Added strum_discriminants derive attributes with consistent snake_case serialization for enum variants
  • Implemented as_db_type() method for both TelemetrySinkConfig and TelemetrySinkConfigDiscriminants for database type string generation
  • Added unit tests in telemetry.rs to verify enum serialization and database type string generation

The changes maintain a clean and consistent API while ensuring proper type safety and database integration.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

(updates since last review)

This PR modifies user ID handling and adds beta access management functionality, with some concerning implementation choices.

  • Removed UserId type alias from user models, reducing type safety for ID format validation in common/src/models/user.rs
  • Added set_beta_access method in admin/src/client.rs with problematic error handling using panic! instead of proper error propagation
  • Added new admin commands SetBetaAccess and UnsetBetaAccess in admin/src/args.rs with basic println logging

5 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

@jonaro00 jonaro00 merged commit 2b11e13 into shuttle-hq:main Feb 4, 2025
21 checks passed
@jonaro00 jonaro00 deleted the telemetry-models branch February 4, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants